Register webhook in codes #332

TommyLike · 2019-07-12T08:10:01Z

For Issue #329, Within this change, admission webhook configs as below are registered in admission codes automatically. Therefore this yaml can be removed.

apiVersion: admissionregistration.k8s.io/v1beta1
kind: ValidatingWebhookConfiguration
metadata:
  name: {{ .Release.Name }}-validate-job
  annotations:
    "helm.sh/hook": pre-install
    "helm.sh/hook-delete-policy": before-hook-creation
webhooks:
  - clientConfig:
      service:
        name: {{ .Release.Name }}-admission-service
        namespace: {{ .Release.Namespace }}
        path: /jobs
    failurePolicy: Ignore
    name: validatejob.volcano.sh
    namespaceSelector: {}
    rules:
      - apiGroups:
          - "batch.volcano.sh"
        apiVersions:
          - "v1alpha1"
        operations:
          - CREATE
          - UPDATE
        resources:
          - jobs
---
apiVersion: admissionregistration.k8s.io/v1beta1
kind: MutatingWebhookConfiguration
metadata:
  name: {{ .Release.Name }}-mutate-job
  annotations:
    "helm.sh/hook": pre-install
    "helm.sh/hook-delete-policy": before-hook-creation
webhooks:
  - clientConfig:
      service:
        name: {{ .Release.Name }}-admission-service
        namespace: {{ .Release.Namespace }}
        path: /mutating-jobs
    failurePolicy: Ignore
    name: mutatejob.volcano.sh
    namespaceSelector: {}
    rules:
      - apiGroups:
          - "batch.volcano.sh"
        apiVersions:
          - "v1alpha1"
        operations:
          - CREATE
        resources:
          - jobs

Also, this two configurations have been added the owner reference of admission service, which makes them deleted automatically when purge helm chart.

TommyLike · 2019-07-12T08:10:40Z

This CI would fail due to the unchange of helm chart.

TravisBuddy · 2019-07-12T08:36:56Z

Travis tests have failed

Hey @TommyLike,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 333d5f20-a480-11e9-bd96-c5bef65e495d

k82cn · 2019-07-12T10:09:57Z

@TommyLike @hzxuzhonghu @asifdxtreme , can you help to list the items we need to do to improve contributor experience for this? That's really hard to get the progress of that if we open PR one by one :(

TravisBuddy · 2019-07-12T10:22:30Z

Travis tests have failed

Hey @TommyLike,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: f240e000-a48e-11e9-bd96-c5bef65e495d

TommyLike · 2019-07-12T23:34:43Z

@TommyLike @hzxuzhonghu @asifdxtreme , can you help to list the items we need to do to improve contributor experience for this? That's really hard to get the progress of that if we open PR one by one :(

hmm, @k82cn this patch is not related to contributor experience improvement. @hzxuzhonghu has opened an issue which lists most of the things we need to do to improve it. We can discuss and talk on that one。#325

k82cn · 2019-07-14T06:14:09Z

this patch is not related to contributor experience improvement.

If contributor can not install Volcano, how to contribute?

TommyLike · 2019-07-15T03:27:58Z

this patch is not related to contributor experience improvement.

If contributor can not install Volcano, how to contribute?

There would be several reasons that leads to failing installing volcano.

kubernetes cluster version requirement.
misconfiguration of the kubernetes cluster
helm binary version requirement.
inconsistency between our charts templates and controller images.
.....

This patch intends to reduce the yaml amount and does not belong to any of these above.

Fixed API definition typo.

TommyLike · 2019-07-15T07:59:33Z

/assign @hzxuzhonghu

cmd/admission/app/configure/configure.go

cmd/admission/main.go

TravisBuddy · 2019-07-15T09:42:19Z

Travis tests have failed

Hey @TommyLike,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: d4c4c390-a6e4-11e9-a3be-7d74802580dc

k82cn · 2019-07-15T12:48:06Z

/lgtm
/approve

volcano-sh-bot · 2019-07-15T12:48:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: k82cn, TommyLike

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [k82cn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

volcano-sh-bot requested review from k82cn and kevin-wangzefeng July 12, 2019 08:10

volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 12, 2019

TommyLike force-pushed the feature/register_webhook_codes branch from 05e4bed to 9c7ae78 Compare July 12, 2019 08:35

TommyLike force-pushed the feature/register_webhook_codes branch from 9c7ae78 to f911c40 Compare July 12, 2019 09:50

TommyLike force-pushed the feature/register_webhook_codes branch from f911c40 to 0876e7d Compare July 15, 2019 02:23

thandayuthapani pushed a commit to thandayuthapani/volcano that referenced this pull request Jul 15, 2019

Merge pull request volcano-sh#332 from k82cn/ka_332

c1c44ca

Fixed API definition typo.

volcano-sh-bot assigned hzxuzhonghu Jul 15, 2019